Skip to content

Feature: views#130

Draft
redweller wants to merge 5 commits into
TheGiddyLimit:developmentfrom
redweller:feature/views
Draft

Feature: views#130
redweller wants to merge 5 commits into
TheGiddyLimit:developmentfrom
redweller:feature/views

Conversation

@redweller
Copy link
Copy Markdown

How to use this feature:

  1. Enable and rename views for current page in the Views dialog (next to Weather settings button)
  2. Assign images or paths to your Views via the context menu
  3. Group of switches in the bottom of layer selection menu on the floating toolbar will allow you to quickly hide or show the items you assigned

1. Enable and rename views for your map in the Views dialog (next to Weather settings button)
2. Assign images or paths to your views via the context menu
3. Group of switches in the bottom of layer selection menu on the floating toolbar will allow you to quickly hide or show the items you assigned
To avoid weird selection behavior
To completely hide items, including light sources & nameplates
Copy link
Copy Markdown
Owner

@TheGiddyLimit TheGiddyLimit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookin cool

Comment thread js/base-engine.js
const model = o.model;
if (!model) continue;
if (`${model.get(query)}`.search(val) > -1) {
const l = model.attributes.layer;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer e.g. const {layer} = model.attributes to randomly one-letter-renaming vars

Comment thread js/base-views.js
$barPage.find(`.handle`).click()
}

function doShowDialog (page) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

things are getting a bit nested here; consider pulling out as d20plus.views._doShowDialog

Comment thread js/base-views.js
];
props.forEach(handleProp);

function doSaveValues () {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer arrow functions const doSaveValues = () => { ... } to avoid clobbering this (and save a few characters!)

Comment thread js/base-views.js
let menuhtml = "";
if (page.get("bR20cfg_viewsEnable")) {
for (let id = 0; id <= 4; id++) {
if (!id || page.get(`bR20cfg_views${id}Enable`)) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider inverting this condition to early-"exit"/reduce nesting;

if (id && !page.get(`bR20cfg_views${id}Enable`)) continue;

const viewname = ...

<hr>
<div>
<div class='pagedetails__header'>
<h3 class='page_title'>Weather</h3>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation's a bit all over the place here; switch it all out for tabs, would you?

@redweller redweller marked this pull request as draft October 8, 2022 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants